New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WW-5370] Makes HttpParameters case-insensitive #807

Merged

lukaszlenart merged 4 commits into master from feature/http-params-case

Dec 12, 2023

+113 −10

Member

lukaszlenart commented Dec 10, 2023

Closes WW-5370


          WW-5370 Makes HttpParameters case-insensitive

13d972d

lukaszlenart changed the title ~~WW-5370 Makes HttpParameters case-insensitive~~ [WW-5370] Makes HttpParameters case-insensitive

lukaszlenart requested review from aleksandr-m, yasserzamani, sdutry, cnenning and kusalk

December 10, 2023 12:15

yasserzamani approved these changes

View reviewed changes

Member

yasserzamani left a comment •

edited

Loading

LGTM, thanks!

P.S. I'm not sure how much it's hard to implement but you might know better: Looks like that same functionality can be achieved with less code by replacing

final private Map<String, Parameter> parameters;

with

final private Map<String, Parameter> parameters = new TreeMap<String, Parameter>(String.CASE_INSENSITIVE_ORDER);

Or using Apache Commons CaseInsensitiveMap.

Member

kusalk commented Dec 11, 2023

Yeah good call - would be better to change the underlying map to be one of the off-the-shelf implementations. The Apache Commons one probably makes more sense.


          WW-5370 Uses TreeMap with case-insensitive comparator

c40978f

Member Author

lukaszlenart commented Dec 12, 2023

Thanks for the ideas! I decided to use TreeMap as proposed by Yasser to avoid introducing another dependency (Commons Collection is used only in tests)

lukaszlenart requested a review from yasserzamani

December 12, 2023 07:54

kusalk reviewed

View reviewed changes

core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java Outdated Show resolved Hide resolved

core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java Outdated Show resolved Hide resolved

core/src/main/java/org/apache/struts2/dispatcher/HttpParameters.java Outdated Show resolved Hide resolved


          WW-5370 Simplifies code

4eaab8a

asfgit force-pushed the feature/http-params-case branch from 7bea060 to 4eaab8a Compare

December 12, 2023 08:48


          WW-5370 Adds proper logic to handle null

102e040

kusalk approved these changes

View reviewed changes

sonarcloud bot commented Dec 12, 2023

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

75.0% Coverage
0.0% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

yasserzamani approved these changes

View reviewed changes

Member

yasserzamani left a comment

Thank you Lukasz! 🙇

lukaszlenart merged commit f684eff into master

9 of 10 checks passed

lukaszlenart deleted the feature/http-params-case branch

December 12, 2023 09:26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kusalk kusalk approved these changes

yasserzamani yasserzamani approved these changes

aleksandr-m Awaiting requested review from aleksandr-m

sdutry Awaiting requested review from sdutry

cnenning Awaiting requested review from cnenning

Labels

None yet